Fix forked repo local SQL Server passwords#3950
Conversation
paulmedynski
added
the
Area\Engineering
There was a problem hiding this comment.
Pull request overview
This PR fixes an issue introduced in #3928 where the removal of a password generation mechanism broke pipeline runs from forked repositories. Forked repos don't have access to Azure DevOps Library variable groups, which contain the $(Password) variable needed for local SQL Server configuration. The fix re-introduces password generation for forked PRs and removes redundant condition parameters from SQL Server configuration templates.
Changes:
- Adds password generation logic for forked repos using
System.PullRequest.IsForkcheck in the test job template - Removes redundant
conditionparameter and its uses from all SQL Server configuration step templates (Windows, Linux, macOS) - Removes condition parameter overrides from the build stage that were calling these templates
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| eng/pipelines/common/templates/jobs/ci-run-tests-job.yml | Adds password generation step for forked repos using GUID; sets Password variable before SQL Server setup |
| eng/pipelines/stages/build-azure-package-ci-stage.yml | Removes condition parameter overrides when calling SQL Server setup templates |
| eng/pipelines/common/templates/steps/configure-sql-server-win-step.yml | Removes condition parameter definition and all condition attributes from steps |
| eng/pipelines/common/templates/steps/configure-sql-server-macos-step.yml | Removes condition parameter definition and condition attribute from step |
| eng/pipelines/common/templates/steps/configure-sql-server-linux-step.yml | Removes condition parameter definition and condition attribute from step |
eng/pipelines/common/templates/steps/configure-sql-server-linux-step.yml
Show resolved
Hide resolved
|
@paulmedynski Happy to help verify this |
|
@ErikEJ - Sure, have at 'er. Can you make a PR based on the branch of this PR and submit it as draft? That should use these changes and let us see how the PR runs perform. |
Codecov Report✅ All modified and coverable lines are covered by tests.
Additional details and impacted files@@ Coverage Diff @@
## main #3950 +/- ##
==========================================
- Coverage 74.53% 67.87% -6.66%
==========================================
Files 266 260 -6
Lines 42915 65704 +22789
==========================================
+ Hits 31987 44599 +12612
- Misses 10928 21105 +10177
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
I'm testing these changes with my own fork via #3952. |
- Re-introduce a pipeline-generated random password for forked builds.
- Addressed Copilot feedback.
eng/pipelines/common/templates/steps/update-config-file-step.yml
Outdated
Show resolved
Hide resolved
paulmedynski
force-pushed
the
dev/paul/db-password
branch
from
a567b28 to
0948c0b
Compare
- Adding diagnostics to help understand why AAD Password auth is failing.
- Removed forced debug flag. - Addressed Copilot comments.
paulmedynski
left a comment
paulmedynski
left a comment
There was a problem hiding this comment.
Commentary for reviewers, and some things for me to fix.
Description
#3928 broke forked pipeline runs by removing the generated password used for local SQL Server configuration. This PR re-introduces it with documentation explaining why it exists :)
Testing